Skip to content

Comments

[4.0] Icon- TO fa ( part 3 )#28075

Merged
rdeutz merged 153 commits intojoomla:4.0-devfrom
N6REJ:icon-
Jul 2, 2020
Merged

[4.0] Icon- TO fa ( part 3 )#28075
rdeutz merged 153 commits intojoomla:4.0-devfrom
N6REJ:icon-

Conversation

@N6REJ
Copy link
Contributor

@N6REJ N6REJ commented Feb 25, 2020

Pull Request for Issue # .

Summary of Changes

This is the final part of the icon to fa- conversions
converts the remaining instances of icon- to fa- so that we're using fontawesome as core icon class.
mappings are not changed

Testing Instructions

using dev inspector check that new and cancel in toolbar use "fas fa-" and do display icons.
check all modals the same way.
ALL icons should now properly use fas fa-

Expected result

image

Actual result

image

Documentation Changes Required

explain how fontawesome is now the defacto iconset for J4+
to mark a icon with the featured color requires adding "featured" to the icon class.
for example,
<i class="fas fa-star"></i> will result in the uncolorized version.
<i class="fas fa-star featured"></i> will colorize it with the standard icon-featured color
if the full class name is sent instead of just the icon name like 'star' it will be passed thru. So you could use 'fas fa-cog' or 'icon-cog' and thats what will be used.

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Feb 25, 2020
@brianteeman
Copy link
Contributor

just going to repeat what was said before - some of these changes are creating a dependency on fontawesome in the front end as well as the admin

@N6REJ
Copy link
Contributor Author

N6REJ commented Feb 25, 2020

just going to repeat what was said before - some of these changes are creating a dependency on fontawesome in the front end as well as the admin

I'm more then open to suggestions, but I think part of it is probably a PLT issue. We have that same dependency on icomoon right now.

@brianteeman
Copy link
Contributor

IIRC @wilsonge said the dependency was ok in the admin but not in the site

We have that same dependency on icomoon right now.

Technically yes but as their code is "icon-file" instead of "fas fa-fw fa-file-0" it is much easier to override the more generic icomoon syntax

@N6REJ
Copy link
Contributor Author

N6REJ commented Feb 25, 2020

IIRC @wilsonge said the dependency was ok in the admin but not in the site

We have that same dependency on icomoon right now.

Technically yes but as their code is "icon-file" instead of "fas fa-fw fa-file-0" it is much easier to override the more generic icomoon syntax

am I actually changing front end icons?
unless I'm sadly mistaken, all icon- will still map. Just our core files will use fa-

@dgrammatiko
Copy link
Contributor

@N6REJ @brianteeman no Font Awesome is not ok for the front end:

joomla/40-backend-template#441 (comment)

So in short:

  • Fields
  • Toolbar
  • etc
    that are shared between FE/BE need to cleaned up from those classes and have the respected icon inlined or in some class in the css. I had Prs for some eg password field...

@N6REJ
Copy link
Contributor Author

N6REJ commented May 27, 2020

@infograf768 could you please retest with your original configuration?

@wilsonge assuming it works for jm, I believe I have it configured now where it will work for all 3rd party extensions & is more flexible. As I stated before if you really want the if's removed I'll revert jgrid & both iconclass.php's to their current 4.0 files. Pretty sure it will break things but...........

@infograf768
Copy link
Member

It now works for me.
I suggest someone tests with another 3rd party component to be sure.

@richard67
Copy link
Member

@N6REJ Your conflict fixes look wrong to me because they seem to undo changes done in the 4.0-dev branch recently for the new workflows.

Please either make sure you update your branch either to the 4.0-dev branch of the CMS repository, or if you update it to your 4.0-dev branch in your repository then make sure that that one is up to date with the 4.0-dev branch of the CMS.

To me it looks as if you have done your conflicts resolution nased on an outdated 4.0-dev branch of your repository.

You can verify that in the changes of this PR: It shows for the files where you just have solved the conflicts more than only the changes you want to make.

N6REJ added 2 commits June 1, 2020 13:32
This reverts commit ef96c0f
This reverts commit 4aa7349
@N6REJ
Copy link
Contributor Author

N6REJ commented Jun 1, 2020

reverted commit fix to double check things per @richard67 statement

# Conflicts:
#	administrator/components/com_workflow/tmpl/stages/default.php
#	administrator/components/com_workflow/tmpl/transitions/default.php
@richard67
Copy link
Member

richard67 commented Jun 1, 2020

@N6REJ I think it looks better now but still I see something which looks as if it had been removed by the new workflows version and now your PR adds it back here:

<td class="nowrap">
<?php
if ($item->condition == 'JARCHIVED'):
$icon = 'icon-archive';
elseif ($item->condition == 'JTRASHED'):
$icon = 'icon-trash';
elseif ($item->condition == 'JPUBLISHED'):
$icon = 'icon-publish';
elseif ($item->condition == 'JUNPUBLISHED'):
$icon = 'icon-unpublish';
endif;
?>
<span class="<?php echo $icon; ?>" aria-hidden="true"></span>
<?php echo Text::_($item->condition); ?>
</td>

@N6REJ
Copy link
Contributor Author

N6REJ commented Jun 1, 2020

grrrrrr it was supposed to remove it.

@richard67
Copy link
Member

Looks ok now regarding being up to date with latest 4.0-dev and conflicts.

@Razzo1987 @Quy @jwaisner Could you give it again a test? Thanks in advance.

# Conflicts:
#	administrator/components/com_templates/src/View/Template/HtmlView.php
@N6REJ
Copy link
Contributor Author

N6REJ commented Jun 8, 2020

@wilsonge Can we get this merged now please?

# Conflicts:
#	libraries/src/Button/FeaturedButton.php
@rdeutz rdeutz merged commit cdbe1ec into joomla:4.0-dev Jul 2, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 2, 2020
brianteeman added a commit to brianteeman/joomla-cms that referenced this pull request Jul 2, 2020
This was referenced Jul 2, 2020
@zero-24 zero-24 added this to the Joomla 4.0 milestone Jul 2, 2020
@N6REJ N6REJ deleted the icon- branch July 3, 2020 14:51
sakiss pushed a commit to sakiss/joomla-cms that referenced this pull request Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.